export classes with pure virtual destructors#53
export classes with pure virtual destructors#53andrurogerz wants to merge 1 commit intocompnerd:mainfrom
Conversation
| // an out-of-line implementation. If a pure virtual destructor is found | ||
| // that has no body and is not defaulted, it must have an out-of-line | ||
| // implementation outside of the translation unit. In this case, treat it | ||
| // like any other out-of-line virtual method decl and export the record. |
There was a problem hiding this comment.
I'm not sure I follow this comment. Why is this the case? Are you referring to the ELF specific behaviour of the virtual dtor serving as a key function? If so, that is an itanium ABI specific behaviour. We can query the C++ model that is in use for evaluating this then.
There was a problem hiding this comment.
No, nothing ABI specific. I will rework this comment so it is more clear.
My intent was to explain that we have two cases for the implementation of a pure virtual destructor. Since pure virtual destructors are the only pure virtual methods that require an implementation (afaict), we know there must be one somewhere. If it is in the translation unit, we don't have to export it:
struct Example {
virtual ~Example() = 0;
};
Example::Example() {}
If it is not in the translation unit, we assume the destructor implementation is in a .cpp file somewhere, so it does need to be exported.
struct Example {
virtual ~Example() = 0
};
Alternatively: we could just ALWAYS export classes that have pure virtual functions. This may lead to exporting a bit more than necessary, but will also catch the case where a class has other pure virtual functions with implementations. Does that sound better?
There was a problem hiding this comment.
Ah, the case now makes more sense! If we can get away with it, that would be nice ... but, doesn't the virtual dtor result in a vtable which also needs to be exported?
There was a problem hiding this comment.
doesn't the virtual dtor result in a vtable which also needs to be exported?
That's correct. This change results in a class getting exported whenever it has a pure virtual dtor.
Having given this more thought, I think we should stick with the more constrained case and export only when there is a pure virtual dtor. If we export a class whenever it contains any pure virtual function, that's going to export a lot more unnecessarily.
Purpose
IDS should annotate classes that have pure virtual destructors with out-of-line implementations. I have observed a small number of these cases while annotating the LLVM codebase and found it was easy to identify and fix automatically with IDS.
Validation